Make InviteReceiptPartnerPolicyPage use new SelectionList
#76279
Conversation
melvin-bot
bot
requested review from
dukenv0307 and
heyjennahay
and removed request for
a team
|
@dukenv0307 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| onSelectRow={() => {}} | ||
| onDismissError={dismissError} | ||
| listItemWrapperStyle={styles.cursorDefault} | ||
| style={{listItemWrapperStyle: styles.cursorDefault}} |
There was a problem hiding this comment.
❌ PERF-4 (docs)
Creating a new object literal {listItemWrapperStyle: styles.cursorDefault} on every render breaks referential equality. If SelectionList or its children are later memoized, this will prevent optimization.
Suggested fix: Move the style object outside the render function or wrap it in useMemo:
const selectionListStyle = useMemo(() => ({
listItemWrapperStyle: styles.cursorDefault,
}), [styles.cursorDefault]);
// Then use:
<SelectionList
style={selectionListStyle}
// ...
/>
| onDismissError={dismissError} | ||
| listItemWrapperStyle={styles.cursorDefault} | ||
| style={{listItemWrapperStyle: styles.cursorDefault}} | ||
| addBottomSafeAreaPadding |
There was a problem hiding this comment.
❌ PERF-4 (docs)
Creating a new textInputOptions object on every render creates unnecessary object allocations. This object contains several properties that change frequently (searchTerm, currentHeaderMessage), so memoization would help prevent unnecessary re-renders if child components are memoized.
Suggested fix: Wrap the textInputOptions object in useMemo:
const textInputOptions = useMemo(() => ({
label: shouldShowTextInput ? translate('common.search') : undefined,
value: searchTerm,
onChangeText: setSearchTerm,
headerMessage: currentHeaderMessage,
}), [shouldShowTextInput, translate, searchTerm, setSearchTerm, currentHeaderMessage]);
// Then use:
<SelectionList
textInputOptions={textInputOptions}
// ...
/>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Not a product change. Removing myself and unsubscribing |
|
@zfurtak Can you please take a look at the failed tests? |
3 participants
Explanation of Change
Fixed Issues
$ #72972
PROPOSAL:
Tests
Note
To test this feature you have to have enabled
UBER_FOR_BUSINESSbetaWorkspacesReceipt partnerstabManage InvitesOffline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2025-11-28.at.14.58.09.mov
MacOS: Desktop